test(e2e): replace vacuous assertions with contract checks - #2261

Description

Problem

A seven-dimension audit of the e2e suite flagged six vacuous or muted tests: assertions that are true by construction (isinstance([], list)), try/except blocks that swallow every outcome, failures downgraded to logger.warning, an or-branch satisfiable from the prompt alone, class-wide xfail(strict=False) that permanently silences whole test classes, and an if guard that hides streaming delta loss. All six passed regardless of whether the contract they claim to test holds.

Solution

Each site is rewritten as an explicit contract check, keeping the file's existing helper/client conventions. Where the contract is genuinely model-dependent (weak tool-callers), the relaxation is routed through the suite's existing, visible mechanisms — FLAKY_TESTS registration or a narrowly-scoped imperative pytest.xfail on only the model-dependent step — never through weakened assertions or silent warnings.

Changes

• e2e_test/chat_completions/test_function_calling.py — test_tool_choice_auto_non_streaming (previously only message is not None) and test_tool_choice_auto_streaming (previously isinstance(list) truisms) now test real auto semantics in both modes: a prompt that clearly needs the declared weather tool ("What's the weather in Tokyo?") must yield a tool call naming a declared tool with JSON-dict args containing the location and finish_reason == "tool_calls"; a prompt that clearly needs none ("What is 2+2? Answer with just the number.") must yield non-empty text, no tool calls, and finish_reason == "stop". Stream accumulation mirrors the sibling test_required_streaming_arguments_chunks_json. Because the auto decision is model-dependent, the two tests are registered in FLAKY_TESTS for TestToolChoiceLlama (Llama-3.2-1B) and TestToolChoiceMistral — matching those classes' existing flaky registrations — which relaxes only the call-vs-answer decision while keeping every produced tool call structurally validated; TestToolChoiceQwen runs fully strict.
• e2e_test/embeddings/test_basic.py — test_embedding_empty_string no longer swallows all outcomes in try/except Exception. It pins an explicit two-branch contract: success must return exactly 1 embedding of the model's dimension (probed with a known-good input), rejection must be a 4xx (openai.APIStatusError / smg_client.ApiError with 400 <= status_code < 500). A 5xx or transport error now fails. git log -L shows the test has swallowed both outcomes since its introduction (refactor(e2e): replace smg_compare with parametrized api_client fixture #812/refactor(e2e): add model fixture, remove deprecated smg fixture #834), so the historically-exhibited behavior is not determinable from history; the comment in the test says so, and the GPU lanes will pin it down.
• e2e_test/router/test_worker_api.py — test_igw_add_and_remove_worker asserts remove_worker success (was logger.warning("Remove worker not supported") on failure) and polls up to 15s before asserting the worker URL is gone from list_workers. No new skip_for_runtime marker was needed: the module already skips vLLM entirely, the class is engine("sglang")-only, and DELETE /workers/{id} is a gateway capability, not backend-dependent.
• e2e_test/responses/test_tools_call.py — test_function_tool_call final assertion drops the or "aquarius" in full_text branch ("Aquarius" appears in the user prompt, so that branch was satisfiable without the tool result ever being consumed). Verified at :411-419 that "baby otter" is exactly the sentinel injected via function_call_output; it is now the sole required token.
• e2e_test/bindings_go/test_go_oai_server.py — per-decision xfail rework:
  • TestGoOAIServerFunctionCalling: class-level xfail(strict=False) removed. The recorded excuse is model capability (Llama-3.2-1B tool reliability), so only the model-dependent steps may xfail, via imperative pytest.xfail(...): (a) model emits no tool call despite tool_choice='required', (b) parsed args are schema-incomplete (missing location). Structural assertions — response shape, streamed index math (0 in tool_calls_by_index), argument accumulation, JSON-dict validity of any tool call the server does parse — are now unconditional, since they verify the Go OAI server's proxy logic, not the model.
  • test_function_calling_tool_choice_none: no xfail at all anymore. tool_choice='none' suppression is a server-side contract a weak tool-caller cannot break; the old class-level marker was silencing it for no reason.
  • TestGoOAIServerMultipleChoices: converted to strict=True. "Go OAI server does not support n > 1" is a deterministic capability gap, not flakiness — both tests fail deterministically while the gap exists, and strict mode makes the XPASS fail CI when support lands, forcing marker removal.
• e2e_test/messages/test_tool_use.py — test_tool_use_streaming: the if input_json_deltas: guard is gone; non-empty deltas and valid-JSON-dict accumulation are asserted unconditionally, mirroring test_mcp_tool.py's assert len(input_json_deltas_by_index) > 0. Also asserts stop_reason == "tool_use" on stream.get_final_message() (a pattern already used in messages/test_tool_search.py). Audit citation note: the claim that "the test already expects stop_reason=='tool_use'" was slightly stale — that assertion lived in the non-streaming siblings (test_single_tool_call), not in this streaming test; it is now asserted here too.

Test Plan

Verified locally (no GPU available):

• python3 -m py_compile passes on all six files.
• ruff check and ruff format --check pass on all six files.
• Full collection with the real conftest — PYTHONPATH=e2e_test:bindings/python/src pytest --collect-only -q over all six files in a Python 3.13 venv (pip install ./e2e_test ./clients/python, with a local stub for the cargo-generated smg_client.types._generated) — collects 226 tests, 0 errors.

Must be confirmed by the GPU lanes (cannot run locally):

• Which branch of the empty-string embedding contract current engines exhibit (200 vs 4xx).
• That Llama-3.2-1B / Mistral tool_choice='auto' behavior stays within the FLAKY_TESTS relaxation and Qwen passes the strict path.
• That worker removal completes within the 15s poll window on the sglang lane.
• That the Go OAI server n>1 tests still fail deterministically (required for the new strict=True xfail) and that the imperative xfails in the function-calling tests trigger only on the model-dependent steps.
• That the Messages streaming path emits input_json deltas (the new unconditional assertion) and the responses tool-call test still finds the "baby otter" sentinel.

Python/CI-only change; cargo gates not applicable.